-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: collect har file on test fail #22526
Conversation
tests/e2e/utils/CheReporter.ts
Outdated
@@ -24,6 +24,7 @@ import { OAUTH_CONSTANTS } from '../constants/OAUTH_CONSTANTS'; | |||
import { REPORTER_CONSTANTS } from '../constants/REPORTER_CONSTANTS'; | |||
import { PLUGIN_TEST_CONSTANTS } from '../constants/PLUGIN_TEST_CONSTANTS'; | |||
import { inject, injectable } from 'inversify'; | |||
const chromeHar = require('chrome-har') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could it be performed using 'import' ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no @types
package for chrome-har
, so I currently use the package like so:
https://github.com/eclipse/che/blob/88e2b95970fb07fb975dbd304c4124cf744380fa/tests/e2e/utils/CheReporter.ts#L29-L30
WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, in this case I am ok with both solutions. We could create own "types" file but I don`t think it is necessary for just one dependency
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good, I have left it at:
// @ts-ignore: No module declaration file
import * as chromeHar from 'chrome-har';
d3ee877
to
88e2b95
Compare
tests/e2e/driver/ChromeDriver.ts
Outdated
@@ -21,6 +21,7 @@ export class ChromeDriver implements IDriver { | |||
|
|||
constructor() { | |||
const options: Options = this.getDriverOptions(); | |||
options.setLoggingPrefs({performance: 'ALL'}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like prettier script was not run before commit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should be fixed in my latest commit
I have fixed the conflict & rebased |
Signed-off-by: David Kwon <[email protected]>
Signed-off-by: David Kwon <[email protected]>
@dkwon17 somehow you commited changes without linting check
|
I will be more careful next time, thank you for the fix: 0811b7e |
What does this PR do?
Fixes #22492
Uses https://www.npmjs.com/package/chrome-har to convert an array of network events into a har file. The har file is saved in the
reports
folder on test failures.Screenshot/screencast of this PR
Viewing har file from
tests/e2e/report/
within Chrome Developer Tools:Screen.Recording.2023-09-25.at.11.47.17.AM.mov
What issues does this PR fix or reference?
#22492
How to test this PR?
tests/e2e
npm ci
.har
file should be available intests/e2e/report/<test-name>/
.har
file under the network tab. The import should be successful, and you should see the list of network requests just like the demo above.PR Checklist
As the author of this Pull Request I made sure that:
What issues does this PR fix or reference
andHow to test this PR
completedReviewers
Reviewers, please comment how you tested the PR when approving it.